feat: add the streaming-reveal extension to @tiptap/ai-toolkit - #8109
feat: add the streaming-reveal extension to @tiptap/ai-toolkit#8109ozdemircibaris wants to merge 18 commits into
Conversation
🦋 Changeset detectedLatest commit: c03e2d5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummaryAdds
WalkthroughAdds ChangesStreaming reveal
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RemoteYjsDoc
participant AiInsertReveal
participant TiptapEditor
RemoteYjsDoc->>AiInsertReveal: Apply remote Yjs insertion
AiInsertReveal->>AiInsertReveal: Track AI-authored inserted run
AiInsertReveal->>TiptapEditor: Schedule decoration render
TiptapEditor->>AiInsertReveal: Resolve relative positions
AiInsertReveal-->>TiptapEditor: Render fading inline decoration
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
area: ai
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ai-toolkit/src/streaming-reveal.spec.ts (1)
157-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider fake timers for the expiry test.
This test relies on real wall-clock delays (30ms duration + 60ms wait) to prove expiry. It'll pass most of the time, but could get flaky on a slow/loaded CI runner.
vi.useFakeTimers()+vi.advanceTimersByTime()(or mockingDate.now) would make it deterministic.As per coding guidelines, "Add or update unit tests for deterministic user-visible behavior, preferring unit tests over end-to-end tests when appropriate."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-toolkit/src/streaming-reveal.spec.ts` around lines 157 - 167, Update the “drops the reveal once its duration has elapsed” test to use Vitest fake timers instead of real setTimeout delays. Enable fake timers before creating the editor, advance them beyond the configured duration to trigger expiry, assert the decoration is removed, and restore real timers afterward.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/ai-toolkit/src/streaming-reveal.spec.ts`:
- Around line 157-167: Update the “drops the reveal once its duration has
elapsed” test to use Vitest fake timers instead of real setTimeout delays.
Enable fake timers before creating the editor, advance them beyond the
configured duration to trigger expiry, assert the decoration is removed, and
restore real timers afterward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d98bc6de-cc92-432c-96ad-6088a795f294
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.changeset/new-pots-admire.mdpackages/ai-toolkit/package.jsonpackages/ai-toolkit/src/streaming-reveal.spec.tspackages/ai-toolkit/src/streaming-reveal.tspackages/ai-toolkit/tsup.config.ts
bdbch
left a comment
bdbch
left a comment
There was a problem hiding this comment.
LGTM just a few nitpicks around comment lengths
d6da91e
coderabbitai
Bot
added
complexity: medium
coderabbitai
Bot
added
complexity: hard
bdbch
left a comment
bdbch
left a comment
There was a problem hiding this comment.
LGTM - just a verification question
| ) => { | ||
| // Only remote edits, i.e. the AI. The local user's own typing is a | ||
| // local transaction and must not fade. | ||
| if (transaction.local) return |
There was a problem hiding this comment.
Is this really only AI? What about collaborative changes that are not local transactions? I think this needs an transaction.origin filter as well to make sure that collab changes don't go through this reveal feature.
There was a problem hiding this comment.
aah good point, it’s not only the AI. local === false just means the update came from applyUpdate, so other collaborators fade too. An origin filter won’t help though: Hocuspocus applies every remote update with the provider as origin, and origin isn’t sent over the wire. Only the clientID on the inserted items can tell AI and human edits apart. I’ll fix the comment for now. We can add clientID filtering if we want it strictly AI only
There was a problem hiding this comment.
Imho we would need to make sure that this is AI only - otherwise collaborative changes from other users would be run through the same reveal logic, right?
There was a problem hiding this comment.
it's AI only right now <3
| } | ||
|
|
||
| /** Limits each revealed streamed insert to guard against mis-resolved positions spanning the document. */ | ||
| const MAX_REVEAL_RANGE = 400 |
There was a problem hiding this comment.
As far as I can see content that exceeds the range just is silently dropping the run (correct me if I'm wrong).
should we just clamp the reveal to those characters and make sure the whole run still completes afterwards?
There was a problem hiding this comment.
You were right, the old code dropped the whole run. fixed thanks!
6711609
coderabbitai
Bot
added
area: core
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai-toolkit/src/streaming-reveal.spec.ts (1)
63-78: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMove this test file under an
__tests__directory.
packages/ai-toolkit/src/streaming-reveal.spec.tsis outside the required test path. Place it underpackages/**/__tests__/to follow the repository test layout.As per coding guidelines, "Use Vitest tests under
packages/**/__tests__/."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-toolkit/src/streaming-reveal.spec.ts` around lines 63 - 78, Move the streaming-reveal test file containing revealDecorations into a packages/**/__tests__/ directory, preserving its existing test contents and behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/ai-toolkit/src/streaming-reveal.spec.ts`:
- Around line 63-78: Move the streaming-reveal test file containing
revealDecorations into a packages/**/__tests__/ directory, preserving its
existing test contents and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e63e3bc8-b957-492e-8c0e-cc16dc6a3533
📒 Files selected for processing (2)
packages/ai-toolkit/src/streaming-reveal.spec.tspackages/ai-toolkit/src/streaming-reveal.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ai-toolkit/src/streaming-reveal.ts
|
@ozdemircibaris I'd like to clarify the open questions before going forward. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai-toolkit/src/streaming-reveal.spec.ts`:
- Around line 1-9: Move the test containing the AiInsertReveal tests from its
current src location into packages/ai-toolkit/__tests__/, then update its
relative AiInsertReveal import to resolve from the new directory while
preserving the existing test setup and cases.
- Around line 214-220: Control timing in the streaming reveal tests: at
packages/ai-toolkit/src/streaming-reveal.spec.ts lines 214-220, enable Vitest
fake timers and set a fixed system time before both remoteInsert calls so their
animation offsets are deterministic; at lines 359-363, replace the real
setTimeout wait with advancing fake timers beyond durationMs, preserving the
existing assertions and cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 609a509b-1029-4362-86e3-260393568105
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.changeset/new-pots-admire.mdpackages/ai-toolkit/package.jsonpackages/ai-toolkit/src/streaming-reveal.spec.tspackages/ai-toolkit/src/streaming-reveal.tspackages/ai-toolkit/tsup.config.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/ai-toolkit/tsup.config.ts
- .changeset/new-pots-admire.md
- packages/ai-toolkit/package.json
- packages/ai-toolkit/src/streaming-reveal.ts
| * The collaboration provider, e.g. `HocuspocusProvider` or `TiptapCloudProvider`. | ||
| * Its awareness is what identifies the AI, so nothing is revealed without it. | ||
| */ | ||
| provider: any |
There was a problem hiding this comment.
Shouldn't we be able to look into the editor options / settings to find out if collaboration is enabled and find the provider from there? That way the user wouldn't need to pass it.
There was a problem hiding this comment.
actually collaboration has a provider option but the extension never reads it and nothing in the repo passes it. So in practice we can only find the provider when CollaborationCaret is used. Not everyone uses the caret, so we would still need the option. I would keep it explicit for now
2 participants
Fixes
N/A (new feature)
Changes and Review
AiInsertRevealextension (@tiptap/ai-toolkit/streaming-reveal): fades in text as the AI streams it into a collaborative document.revealAiText) orstreaming-reveal.spec.ts.Checklist
Responsibility